Edit Post: Refactor effects to action generators #27069

tyxla · 2020-11-18T13:10:09Z

Description

This PR migrates the edit-post store effects into action generators. See #25643.

That allows us to also remove refx as a middleware for side effects, which this PR is also taking care of.

How has this been tested?

Unit test modifications - ensuring they pass.
Manual testing of various post editor states to ensure behavior hasn't changed
- Switching the editor mode from visual to code continues to deselect selected blocks.
- With "Custom Fields" from the advanced options panel enabled, ensuring meta fields still work correctly.
The e2e tests continue to pass

Types of changes

Non-breaking code quality enhancement.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-11-18T13:20:33Z

Size Change: -61 B (0%)

Total Size: 1.2 MB

Filename	Size	Change
`build/block-directory/index.js`	8.72 kB	+7 B (0%)
`build/block-editor/index.js`	134 kB	+2 B (0%)
`build/block-library/index.js`	148 kB	-3 B (0%)
`build/blocks/index.js`	48.1 kB	+2 B (0%)
`build/components/index.js`	172 kB	-7 B (0%)
`build/core-data/index.js`	14.8 kB	+2 B (0%)
`build/data-controls/index.js`	828 B	+1 B
`build/data/index.js`	8.8 kB	+4 B (0%)
`build/date/index.js`	11.2 kB	-1 B
`build/edit-navigation/index.js`	11.1 kB	-2 B (0%)
`build/edit-post/index.js`	306 kB	-81 B (0%)
`build/edit-site/index.js`	24 kB	+3 B (0%)
`build/edit-widgets/index.js`	26.3 kB	+2 B (0%)
`build/editor/index.js`	43.3 kB	+2 B (0%)
`build/format-library/index.js`	6.87 kB	+3 B (0%)
`build/list-reusable-blocks/index.js`	3.1 kB	+1 B
`build/media-utils/index.js`	5.32 kB	+1 B
`build/redux-routine/index.js`	2.84 kB	+1 B
`build/server-side-render/index.js`	2.77 kB	+1 B
`build/url/index.js`	4.06 kB	+1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.8 kB	0 B
`build/api-fetch/index.js`	3.42 kB	0 B
`build/autop/index.js`	2.84 kB	0 B
`build/blob/index.js`	664 B	0 B
`build/block-directory/style-rtl.css`	943 B	0 B
`build/block-directory/style.css`	942 B	0 B
`build/block-editor/style-rtl.css`	11.3 kB	0 B
`build/block-editor/style.css`	11.3 kB	0 B
`build/block-library/editor-rtl.css`	8.95 kB	0 B
`build/block-library/editor.css`	8.96 kB	0 B
`build/block-library/style-rtl.css`	8.23 kB	0 B
`build/block-library/style.css`	8.23 kB	0 B
`build/block-library/theme-rtl.css`	792 B	0 B
`build/block-library/theme.css`	793 B	0 B
`build/block-serialization-default-parser/index.js`	1.87 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.06 kB	0 B
`build/components/style-rtl.css`	15.3 kB	0 B
`build/components/style.css`	15.3 kB	0 B
`build/compose/index.js`	9.95 kB	0 B
`build/deprecated/index.js`	768 B	0 B
`build/dom-ready/index.js`	571 B	0 B
`build/dom/index.js`	4.95 kB	0 B
`build/edit-navigation/style-rtl.css`	881 B	0 B
`build/edit-navigation/style.css`	885 B	0 B
`build/edit-post/style-rtl.css`	6.42 kB	0 B
`build/edit-post/style.css`	6.4 kB	0 B
`build/edit-site/style-rtl.css`	3.92 kB	0 B
`build/edit-site/style.css`	3.92 kB	0 B
`build/edit-widgets/style-rtl.css`	3.13 kB	0 B
`build/edit-widgets/style.css`	3.13 kB	0 B
`build/editor/editor-styles-rtl.css`	476 B	0 B
`build/editor/editor-styles.css`	478 B	0 B
`build/editor/style-rtl.css`	3.85 kB	0 B
`build/editor/style.css`	3.85 kB	0 B
`build/element/index.js`	4.62 kB	0 B
`build/escape-html/index.js`	735 B	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	2.27 kB	0 B
`build/html-entities/index.js`	623 B	0 B
`build/i18n/index.js`	3.57 kB	0 B
`build/is-shallow-equal/index.js`	698 B	0 B
`build/keyboard-shortcuts/index.js`	2.54 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	476 B	0 B
`build/list-reusable-blocks/style.css`	476 B	0 B
`build/notices/index.js`	1.81 kB	0 B
`build/nux/index.js`	3.42 kB	0 B
`build/nux/style-rtl.css`	671 B	0 B
`build/nux/style.css`	668 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.43 kB	0 B
`build/priority-queue/index.js`	790 B	0 B
`build/reusable-blocks/index.js`	2.92 kB	0 B
`build/rich-text/index.js`	13.3 kB	0 B
`build/shortcode/index.js`	1.69 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/viewport/index.js`	1.86 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.22 kB	0 B

_{compressed-size-action}

adamziel · 2020-11-23T15:43:36Z

@tyxla it seems like we overlapped in our efforts quite a lot: #27006 ; Your PR passed more tests though and looks closer to being merge-able so let's continue here.

adamziel · 2020-11-23T15:50:12Z

packages/edit-post/src/store/actions.js

+	);
+
+	// First remove any existing subscription in order to prevent multiple saves
+	if ( !! saveMetaboxUnsubscribe ) {


For this to work you'd need to declare let saveMetaboxUnsubscribe outside of this function, right now this will always be false since the variable is undefined at this point. I think this doesn't throw an error only because const is transpiled to var.

BTW this won't work with multiple registries. However, effects.js wouldn't as well so I don't think it's a blocker - just something to take note of for later.

I updated that in 2e12440

Thanks, @adamziel, that makes sense 👍

adamziel · 2020-11-23T15:55:48Z

This looks good overall, I left a few comments and there are some more tests failing, but it seems pretty close.

Edit: Interestingly, in #27232 all the tests failed on CI. Still, the ones failing here worked in my local environment. I wonder what's the root cause.

tyxla · 2020-11-25T13:39:22Z

This looks good overall, I left a few comments and there are some more tests failing, but it seems pretty close.

Edit: Interestingly, all the tests failed on CI. Still, the ones failing here worked in my local environment. I wonder what's the root cause.

Thanks for taking a look @adamziel. I've been busy with other things these past couple of days, but I'll make sure to devote some time to this one tomorrow (Thursday).

adamziel · 2020-11-26T11:18:15Z

@tyxla interestingly, the tests in #27232 fail because wp.data.dispatch( 'core/edit-post' ).toggleFeature( 'welcomeGuide' ) is no longer in effect after refreshing the page. Not sure why though.

Edit: I think this was the culprit: 98a88a8

tyxla · 2020-11-26T11:35:21Z

FWIW we were missing the controls in the registry, but I addressed that in b983bd4 and that resolved one of the failing e2e tests. Looking into the other one now.

tyxla · 2020-11-26T11:42:23Z

@adamziel I think the problem was that with the new action generators the requestMetaBoxUpdates will yield a META_BOX_UPDATES_SUCCESS action at the end, and in the e2e test, we check if the publish button is disabled after that. This is a problem; we should be checking when requestMetaBoxUpdates has started, but hasn't finished yet. So I've altered the e2e test to do the assertion earlier in 3470c38.

Let's see if the tests will pass this time 😉

tyxla · 2020-11-26T12:15:12Z

@adamziel I think the problem was that with the new action generators the requestMetaBoxUpdates will yield a META_BOX_UPDATES_SUCCESS action at the end, and in the e2e test, we check if the publish button is disabled after that. This is a problem; we should be checking when requestMetaBoxUpdates has started, but hasn't finished yet. So I've altered the e2e test to do the assertion earlier in 3470c38.

Let's see if the tests will pass this time 😉

They're green ✅

adamziel · 2020-11-26T12:29:33Z

packages/edit-post/src/store/index.js

-// We shouuld be able to make the switch once we remove the effects.
-const instantiatedStore = registerStore( STORE_NAME, storeConfig );
-applyMiddlewares( instantiatedStore );
+registerStore( STORE_NAME, storeConfig );


Ideally we'd find a way to use register() here, but it's not a blocker for this PR

adamziel · 2020-11-26T12:30:14Z

Great work @tyxla, Thank you!

tyxla · 2020-11-26T12:30:51Z

Thanks for all the help and reviews @adamziel 🙌

tyxla added [Type] Enhancement A suggestion for improvement. [Type] Code Quality Issues or PRs that relate to code quality [Package] Edit Post /packages/edit-post labels Nov 18, 2020

tyxla requested review from youknowriad, jsnajdr and nerrad November 18, 2020 13:10

tyxla requested review from ajitbohra, chrisvanpatten and mkaz as code owners November 18, 2020 13:10

tyxla self-assigned this Nov 18, 2020

tyxla force-pushed the refactor/edit-post-store-action-generators branch from 53882b9 to 4d8cf8e Compare November 18, 2020 14:46

youknowriad requested a review from adamziel November 18, 2020 19:13

adamziel mentioned this pull request Nov 23, 2020

Try to remove effects.js middleware #27006

Closed

adamziel reviewed Nov 23, 2020

View reviewed changes

adamziel mentioned this pull request Nov 24, 2020

Edit Post: Replace effects.js with controls.js #27232

Closed

tyxla and others added 8 commits November 26, 2020 13:33

Edit Post: Refactor effects to action generators

f83ad94

Edit Post: Remove effects middleware and use register()

9569b6a

Edit Post: Remove refx and add @wordpress/data-controls

646b98b

Edit Post: Delete unused effects and middlewares

6accb23

Edit Post: Fix tests

c36eccb

Edit Post: Revert store registration

c4fb951

Use global saveMetaboxUnsubscribe

b705ada

Enable the data controls in the edit-post registry

28e439b

tyxla force-pushed the refactor/edit-post-store-action-generators branch from b983bd4 to 28e439b Compare November 26, 2020 11:33

Do not wait for requestMetaBoxUpdates to finish

3470c38

tyxla requested review from ntwb and talldan as code owners November 26, 2020 11:40

tyxla requested a review from adamziel November 26, 2020 12:15

adamziel reviewed Nov 26, 2020

View reviewed changes

adamziel approved these changes Nov 26, 2020

View reviewed changes

adamziel merged commit 8f5f35f into master Nov 26, 2020

adamziel deleted the refactor/edit-post-store-action-generators branch November 26, 2020 12:30

github-actions bot added this to the Gutenberg 9.5 milestone Nov 26, 2020

adamziel mentioned this pull request Nov 26, 2020

Block editor: refactor effect.js to controls #27298

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Edit Post: Refactor effects to action generators #27069

Edit Post: Refactor effects to action generators #27069

tyxla commented Nov 18, 2020 •

edited

Loading

github-actions bot commented Nov 18, 2020 •

edited

Loading

adamziel commented Nov 23, 2020 •

edited

Loading

adamziel Nov 23, 2020 •

edited

Loading

adamziel Nov 23, 2020 •

edited

Loading

adamziel Nov 24, 2020

tyxla Nov 24, 2020

adamziel commented Nov 23, 2020 •

edited

Loading

tyxla commented Nov 25, 2020

adamziel commented Nov 26, 2020 •

edited

Loading

tyxla commented Nov 26, 2020

tyxla commented Nov 26, 2020 •

edited

Loading

tyxla commented Nov 26, 2020

adamziel Nov 26, 2020 •

edited

Loading

adamziel commented Nov 26, 2020

tyxla commented Nov 26, 2020

Edit Post: Refactor effects to action generators #27069

Edit Post: Refactor effects to action generators #27069

Conversation

tyxla commented Nov 18, 2020 • edited Loading

Description

How has this been tested?

Types of changes

Checklist:

github-actions bot commented Nov 18, 2020 • edited Loading

adamziel commented Nov 23, 2020 • edited Loading

adamziel Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

adamziel Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

adamziel Nov 24, 2020

Choose a reason for hiding this comment

tyxla Nov 24, 2020

Choose a reason for hiding this comment

adamziel commented Nov 23, 2020 • edited Loading

tyxla commented Nov 25, 2020

adamziel commented Nov 26, 2020 • edited Loading

tyxla commented Nov 26, 2020

tyxla commented Nov 26, 2020 • edited Loading

tyxla commented Nov 26, 2020

adamziel Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

adamziel commented Nov 26, 2020

tyxla commented Nov 26, 2020

tyxla commented Nov 18, 2020 •

edited

Loading

github-actions bot commented Nov 18, 2020 •

edited

Loading

adamziel commented Nov 23, 2020 •

edited

Loading

adamziel Nov 23, 2020 •

edited

Loading

adamziel Nov 23, 2020 •

edited

Loading

adamziel commented Nov 23, 2020 •

edited

Loading

adamziel commented Nov 26, 2020 •

edited

Loading

tyxla commented Nov 26, 2020 •

edited

Loading

adamziel Nov 26, 2020 •

edited

Loading